Pass pydantic validation context #3448
Conversation
Add tests involving the new 'validation context' for: - Pydantic model as the output type - Tool, native and prompted output - Tool calling - Output function - Output validator
| try: | ||
| self._next_node = await self._handle_text_response(ctx, text, text_processor) | ||
| self._next_node = await self._handle_text_response( | ||
| ctx, ctx.deps.tool_manager.validation_ctx, text, text_processor |
There was a problem hiding this comment.
Since we already pass in ctx, we don't need a new arg do we?
There was a problem hiding this comment.
Would it make sense to store the validation context directly on ctx.deps, instead of going through the tool manager?
| self, | ||
| data: str, | ||
| run_context: RunContext[AgentDepsT], | ||
| validation_context: Any | None, |
There was a problem hiding this comment.
Let's have a default of None
There was a problem hiding this comment.
Also while we're at it, let's require kwargs for everything after data, by inserting , *,
| model_settings: Optional model request settings to use for this agent's runs, by default. | ||
| retries: The default number of retries to allow for tool calls and output validation, before raising an error. | ||
| For model request retries, see the [HTTP Request Retries](../retries.md) documentation. | ||
| validation_context: Additional validation context used to validate all outputs. |
There was a problem hiding this comment.
Let's link to the Pydantic doc
| result_data = await text_processor.process(text, run_context, validation_context) | ||
|
|
||
| for validator in ctx.deps.output_validators: | ||
| result_data = await validator.validate(result_data, run_context) |
There was a problem hiding this comment.
Ideally, output validators would have access to the validation context as well, as they could call model_validate themselves. They already have access to RunContext, so maybe it'd make sense to store the validation context on there? As the validation context callable itself needs RunContext, building run context could look like ctx = <run context>; validation_ctx = callable(ctx); ctx = replace(ctx, validation_ctx = validation_ctx)
I think that'd be a refactor worth exploring that could allow us to drop a lot of the new arguments.
|
Thanks @DouweM for the feedback! Do you prefer small commits integrating your remarks one-by-one or a single large commit with everything ? I don't know what's more comfortable to review (or if it matters at all) ps: 'yes' to all your suggestions/comments aha |
…ta` in process functions
@NicolasPllr1 Doesn't really make a difference, I typically review either the diff since last time, or more frequently the entire diff with main again. |
| validation_context = build_validation_context(ctx.deps.validation_context, run_context) | ||
|
|
||
| result_data = await text_processor.process(text, run_context) | ||
| run_context = replace(run_context, validation_context=validation_context) |
There was a problem hiding this comment.
Would it make sense to move this into build_run_context?
| messages: list[_messages.ModelMessage] = field(default_factory=list) | ||
| """Messages exchanged in the conversation so far.""" | ||
| validation_context: Any = None | ||
| """Additional Pydantic validation context for the run outputs.""" |
There was a problem hiding this comment.
Let's not call it "additional", and let's link to the Pydantic validation context docs.
Would also be good to mention this feature in docs somewhere :)
| model_settings: Optional model request settings to use for this agent's runs, by default. | ||
| retries: The default number of retries to allow for tool calls and output validation, before raising an error. | ||
| For model request retries, see the [HTTP Request Retries](../retries.md) documentation. | ||
| validation_context: Additional [validation context](https://docs.pydantic.dev/latest/concepts/validators/#validation-context) used to validate all outputs. |
There was a problem hiding this comment.
Ah here's the link! Let's explicitly call it Pydantic validation context, instead of "Additional"
|
@DouweM ready for the another round of review ; ) Thanks again for the nice and precise feedback you provided |
2 participants
This PR adds a new
validation_contextparameter to theAgent.This object is either
Anyor a callable on the agent's run context returningAny. It is fed to the Pydantic validation of all outputs as additional contextRelated issue